London|26-ITP-January|Alexandru Pocovnicu|Sprint 3| alarm clock#1057
London|26-ITP-January|Alexandru Pocovnicu|Sprint 3| alarm clock#1057alexandru-pocovnicu wants to merge 12 commits intoCodeYourFuture:mainfrom
Conversation
alexandru-pocovnicu
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Code works fine if a user only clicks the "Set Alarm" button once.
However, if the user enters a time and then clicks the "Set Alarm" button multiple times, the countdown clock will not display properly.
Can you fix the issue?
Currently when starting a new countdown, the application does not always return to a clean initial state, which can lead to inconsistent behaviour between runs.
Consider introducing a dedicated reset function to return the app to a clean initial state to help ensure consistency.
| function setAlarm() { | ||
| const alarmSetEl = document.getElementById("alarmSet"); | ||
| const timeRemainingEl = document.getElementById("timeRemaining"); | ||
| let totalSeconds = +alarmSetEl.value; |
There was a problem hiding this comment.
Some unusual input values can make your app behave abnormally. Can you add code to sanitise them?
cjyuan
added
Reviewed
|
when i press stopalarm i want the countdown to go to 0,call pauseAlarm(), and the class to be removed from the body,but on line 48 i have this message // DO NOT EDIT BELOW HERE, I can't really think of a way of doing it without adding code bellow that |
alexandru-pocovnicu
added
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Note: To reset countdown display when the user clicks "Stop", and without modifying existing code, you can add another event listener to the stop button:
document.getElementById("stop").addEventListener("click", () => {
// Your code here ...
});
| isNaN(totalSeconds) || | ||
| !Number.isInteger(totalSeconds) | ||
| ) { | ||
| alarmSetEl.value = null; |
There was a problem hiding this comment.
.value property is of type "string". Setting its value to "" is better.
| let paddedSeconds = seconds.toString().padStart(2, "0"); | ||
| let paddedMinutes = minutes.toString().padStart(2, "0"); | ||
| timeRemainingEl.innerHTML = `Time Remaining: ${paddedMinutes}:${paddedSeconds}`; | ||
| alarmSetEl.value = null; |
There was a problem hiding this comment.
Why clear the input field when updating the timer display? When a countdown is active, it will be cleared every second.
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
implemented an alarm clock that counts down from the input value